Integrate context/name schema into program context - #179
Conversation
Wire the existing but unreferenced context/name.schema.yaml into program/context.schema.yaml using the same if/then pattern as other context properties. Update the name schema description to clarify its role in disambiguating pick alternatives. Add a named pick example to demonstrate the intended usage.
|
gnidan
left a comment
gnidan
left a comment
There was a problem hiding this comment.
Writer review — descriptions and example quality:
context/name.schema.yaml description: The rewrite is a clear improvement. "A label for this context" is better than "The name of the context", and the pick tie-in gives concrete motivation. The "opaque strings" paragraph is a good addition. One small suggestion: "identifying it from other contexts" reads slightly awkwardly — consider "distinguishing it from other contexts" instead.
context.schema.yaml if/then clause: The inline description says "A label for this context, for use in identifying it from other contexts" — this duplicates the first sentence of the name schema's own description almost verbatim. The other if/then descriptions in this file are shorter and describe the role of the property rather than restating the subschema description. Consider something briefer like:
description: |
An optional label for disambiguating this context from others.This matches the style of the neighboring entries ("The context's corresponding source code range", "Variable definitions, types, allocations known to exist in the context", etc.).
pick example: Good addition — the name properties make the disambiguation use case concrete. The # example: comment follows conventions. The example pairs well with the existing unnamed pick example, showing the before/after pattern. Looks good.
name example: Adding "inlined-call" is a nice complement to the existing "Array<T=bytes32>" — shows both a simple label and a more complex parameterized name. Consider adding # example: comments to both:
examples:
# example: simple label for an inlined call site
- name: "inlined-call"
# example: parameterized name for a generic instantiation
- name: "Array<T=bytes32>"Overall the descriptions are clear and the examples effectively illustrate the use case. The suggestions above are minor polish.
gnidan
left a comment
gnidan
left a comment
There was a problem hiding this comment.
Docs infrastructure review:
Schema index gap: schema:ethdebug/format/program/context/name is not in packages/web/src/schemas.ts. The programSchemaIndex lists ["code", "variables", "remark", "pick", "gather", "frame"] but not "name". This means:
- When the context schema viewer encounters the
$reftocontext/name, it will render the schema inline instead of showing a "See documentation" link - This is functional but inconsistent with how the other context sub-schemas appear
No spec page exists: There's no packages/web/spec/program/context/name.mdx file. All other context types (code.mdx, variables.mdx, remark.mdx, pick.mdx, gather.mdx, frame.mdx) have dedicated spec pages. Without one, even adding name to the schema index would produce a broken link.
Recommendation: This PR is fine as-is for the schema changes — the SchemaViewer will render context/name inline and the examples will display correctly. But for completeness, a follow-up should:
- Add
spec/program/context/name.mdx(matching the pattern of sibling files) - Add
"name"to the array inschemas.tsline 227
Not blocking — the site will build and work correctly without these. The inline rendering is adequate for now.
- Use "distinguishing" instead of "identifying it from" - Shorten inline description in context.schema.yaml to match sibling entries' style - Add example comments to name.schema.yaml examples
Add name.mdx doc page following the pattern of sibling context pages (remark, frame, etc.). Register the schema ID in schemas.ts so the spec site links to the new page.
|
Note: the spec page and schemas.ts entry were added to this PR per maintainer decision. Inline schema rendering (without a dedicated spec page) is not acceptable — all new schemas must include their spec page in the same PR, not as a follow-up. |
1 participant
The
context/name.schema.yamlfile has existed since the initial program schema commit but was never wired intoprogram/context.schema.yaml. This PR integrates it using the sameif/thenpattern as the other context properties (code,variables,remark,pick,gather,frame).Changes:
if: required: ["name"]/then: $ref context/nameclause toallOf, makingnamean optional property on any context object.pickalternatives at runtime.pick.This is purely additive and non-breaking. The
nameproperty becomes available on any context but remains optional. No TypeScript changes needed — the schema validation tests pass with the new examples.